Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RELAY] IR builder stablize refactor, clean pass #1934

Merged
merged 2 commits into from
Oct 20, 2018
Merged

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Oct 19, 2018

This is a clean refactor PR to attempt stablize the relay python IR building API, it also contains fixes and patches to enable text format.

One of the general guidelines behind this refactor are:

  • Minimum abstraction when possible (simplification of IRBuilder)
  • Python API consistency (env.merge->env.update)
  • Make ir_pass style consistent
    • Expr is always the first argument
    • Not require env to be present, when necessary, pass as a second argument

List of Changes

  • Simplify ir_builder to scope_builder cc @jroesch
    • ScopeBuilder is only used to build if and let scopes.
    • Use relay.Function to build functions (single way to build functions)
  • Rewrite all the test cases with the new style.
  • Add text print test to each op, fix problems that surface due to the new testcases.
  • Improve the BroadcastRel to handle symbolic case cc @jroesch
  • Fix TupleWrapper cc @slyubomirsky
    • To make sure python unpack works, we need to throw index error when there is out of bound access
    • TupleWrapper cannot assume the input field is a tuple type
  • Enable TulpleGetItem type inference with incomplete type
  • Fix dead code elimination when type is bool cc @MarisaKirisame
  • Fix LeakyReluAttrs cc @siju-samuel
  • Merge reduce tests into a more concise functioncc @siju-samuel
  • Rename TypeParam->TypeVar as suggested by @joshpoll

@tqchen
Copy link
Member Author

tqchen commented Oct 19, 2018

@jroesch @MarisaKirisame @junrushao1994 @slyubomirsky please review

@junrushao
Copy link
Member

Will do tonight

this->global_map_.Set(str, id);
return id;
}
GlobalVar EnvironmentNode::GetGlobalVar(const std::string &name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const std::string& name

CHECK(type.as<IncompleteTypeNode>() == nullptr);
if (functions.find(var) != functions.end()) {
if (!update) {
throw dmlc::Error("already have definition for XXXX.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do XXXX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants